Skip to content

Conversation

trodi
Copy link
Contributor

@trodi trodi commented Jul 14, 2016

Previously limit labels would be hidden based on the position of the pointer labels even if user
options hide pointer labels.

This addresses #353

… hidden

Previously limit labels would be hidden based on the position of the pointer labels even if user
options hide pointer labels
@codecov-io
Copy link

Current coverage is 100%

Merging #373 into master will not change coverage

@@           master   #373   diff @@
====================================
  Files           1      1          
  Lines         842    844     +2   
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
+ Hits          842    844     +2   
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last updated by f947823...d3567ce

@ValentinH
Copy link
Member

Thanks for this PR, it looks good.

I'm wondering if, when the hidePointerLabels is toggled at runtime, everything will work as expected. Indeed, if the option is off and the floor/ceil label is hidden because the pointer is next to it, and the option is turned on, the floor/ceil label will stay hidden while they should be visible...

@ValentinH
Copy link
Member

I'm away from computer until the end of the weekend so I'll try this scenario later. ;)

@trodi
Copy link
Contributor Author

trodi commented Jul 14, 2016

Here's a JSFiddle which should demonstrate your use case. I believe it handles as expected. Below are concrete steps to test (as I understood them).

  1. hidePointerLabels: false
  2. move point to force min limit to hide
  3. programmatically toggle hidePointerLabels: true
  4. min label is visible and pointer label is hidden

@ValentinH
Copy link
Member

Indeed, this corresponds to what I was talking about. Good job! ;)

@ValentinH ValentinH merged commit 012b0f0 into angular-slider:master Jul 15, 2016
@trodi trodi deleted the bug-fix-hidePointerLabels branch July 15, 2016 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants